-
-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ISSUE-919: Add a decadent draft game mode #990
Conversation
Tried on the beta build, but it kept crashing |
Thanks for giving it a shot! Can you share what settings you used? Is there any way to get the logs? There don't seem to be any integration/UI tests and I'm not totally sure what the best way to manually test is. |
I tested Decadent mode, 36 packs for both Ikoria and Theros Returned, but they both crashed when I tried to start the game. Unfortunately, I don't have a way to grab those logs from the service. Did it work when you ran it locally under the same conditions? |
Worked for me with Guilds of Ravnica. But I realize just now that I had # players set to 1. Once I adjusted to 8 it crashed! Looking into it, and converting this to a draft PR. Thanks for your patience! |
Worked for IKO and THB for me. Tried 36 and 60 packs. 8 player.
|
I'm on Windows 10. From Firefox web console:
From the Firefox browser console:
Something else I can check? Edit: It's something specific to this PR, as the deplyoment for #1020 works just fine in my FF75.0 (64bit) on Win10 64bit. |
I think one can easily argument either way. For my understanding I tend to say Decadent is a subtype, but in the end it's the way you put or name it. Note: Currently, a "Decadent Sealed" (sometimes offered even at GP level as side event!) is also not possible. If one would call the game mode where every player start with a complete booster box (36 booster) as such.
Elaborating your idea even further, the result might be an additional "layer" and could look something like this:
Decadent, Glimpse, Rochester etc. are just picking variations for the limited game mode "draft". For either of those the underlying card pool could be one of several options, too. IMO we should keep this discussion out of this PR, finish the feature and find the best UI solution to approach further game modes in an issue ticket. |
I like tom's idea of a 3rd category, although it might add some excessive button clicks to the program. I think it's fine as-is as a separate option that appears when draft is selected. Lets roll w/ it, and if there are issues in the future, we can reevaluate this. |
@tooomm Thanks for your input on the Game Type + Pool Type + Picking Type. Agree that discussion should be moved out of this PR and into an issue. Thanks also for the details about the error you're seeing. I have a hunch that this will fix it. a182b91 Explanation of my hunch: If this does not work then this will require more digging. Unfortunately I don't have access to a Windows 10 machine, so I'd rely on your help here. Perhaps using |
Solution: 1. Implement glimpse draft as a superset of regular draft a. human.js - Make autopick into a list of picks to make - Do pick after pick, removing elements from this list - On new cycle of picks, restore autopick list to initial value - On itmeout, go through the list and for each `null`, make a random pick - Make pick function declarative so that it's easy to understand what's going on b. game.js - Implement a new pack action called "keep", it does the same thing "pass" does, except it doesn't actually pass the pack. - Have c. bot.js - Make it comply with the new semantics d. frontend/* and util.js - Make necessary changes for the game to recognize this new mode 2. Make draft functionality, such as `pick` function family polymorphic. This turned out to not be needed for this PR (even though I was using this actively during development[1]), but it's something that was considered in a sister-PR[2] and I preserved in this PR. a. human.js - Use static methods per necessity (so that Human instances can be parametrized by Human functions from Game class at construction time) b. game.js - Add mk_draft_fns function that can later be used in order to have more esoteric pick functions in a decoupled way (for instance, we can implement Grid draft like that by just implementing JSX and two functions in human.js!) Automated testing was performed and no regressions were detected. No new tests were written since this PR only slightly extends the old functionality and no regressions is a good indicator of correctness in this case. Thorough manual testing was performed. To understand the process of development better, one can consult development branch, which shall NOT be deleted, because relics of programming should be preserved for insights into design decisions and approaches that didn't work.[1] --- [1]: https://github.com/serokellcao/dr4ft/commits/glimpse-draft-relic [2]: dr4fters#990 (comment)
Solution: 1. Implement glimpse draft as a superset of regular draft a. human.js - Make autopick into a list of picks to make - Do pick after pick, removing elements from this list - On new cycle of picks, restore autopick list to initial value - On itmeout, go through the list and for each `null`, make a random pick - Make pick function declarative so that it's easy to understand what's going on b. game.js - Implement a new pack action called "keep", it does the same thing "pass" does, except it doesn't actually pass the pack. - Have c. bot.js - Make it comply with the new semantics d. frontend/* and util.js - Make necessary changes for the game to recognize this new mode 2. Make draft functionality, such as `pick` function family polymorphic. This turned out to not be needed for this PR (even though I was using this actively during development[1]), but it's something that was considered in a sister-PR[2] and I preserved in this PR. a. human.js - Use static methods per necessity (so that Human instances can be parametrized by Human functions from Game class at construction time) b. game.js - Add mk_draft_fns function that can later be used in order to have more esoteric pick functions in a decoupled way (for instance, we can implement Grid draft like that by just implementing JSX and two functions in human.js!) Automated testing was performed and no regressions were detected. No new tests were written since this PR only slightly extends the old functionality and no regressions is a good indicator of correctness in this case. Thorough manual testing was performed. To understand the process of development better, one can consult development branch, which shall NOT be deleted, because relics of programming should be preserved for insights into design decisions and approaches that didn't work.[1] --- [1]: https://github.com/serokellcao/dr4ft/commits/glimpse-draft-relic [2]: dr4fters#990 (comment)
Solution: 1. Implement glimpse draft as a superset of regular draft a. human.js - Make autopick into a list of picks to make - Do pick after pick, removing elements from this list - On new cycle of picks, restore autopick list to initial value - On itmeout, go through the list and for each `null`, make a random pick - Make pick function declarative so that it's easy to understand what's going on b. game.js - Implement a new pack action called "keep", it does the same thing "pass" does, except it doesn't actually pass the pack. - Choose the correct static handlers (callbacks) for the format based on `mkDraftFns` switch. c. bot.js - Make it comply with the new semantics d. frontend/* and util.js - Make necessary changes for the game to recognize this new mode 2. Make draft functionality, such as `pick` function family polymorphic. This turned out to not be needed for this PR (even though I was using this actively during development[1]), but it's something that was considered in a sister-PR[2] and I preserved in this PR. a. human.js - Use static methods per necessity (so that Human instances can be parametrized by Human functions from Game class at construction time) b. game.js - Add mk_draft_fns function that can later be used in order to have more esoteric pick functions in a decoupled way (for instance, we can implement Grid draft like that by just implementing JSX and two functions in human.js!) Automated testing was performed and no regressions were detected. No new tests were written since this PR only slightly extends the old functionality and no regressions is a good indicator of correctness in this case. Thorough manual testing was performed. To understand the process of development better, one can consult development branch, which shall NOT be deleted, because relics of programming should be preserved for insights into design decisions and approaches that didn't work.[1] --- [1]: https://github.com/serokellcao/dr4ft/commits/glimpse-draft-relic [2]: dr4fters#990 (comment)
@keeler Awesome, that did the trick. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Last check @HerveH44?
Well I still see open my points on the delegate functions being in the game.join function. I don't know if @keeler looked at it or not. I would prefer to go back to implement it to the "pick" function of the player like it was before, because the delegate functions theorically should be pure functions, out in their own file so they can be testable. But this can be done in a refactor, now I guess it's time to move on and let the other PR merge this one. |
@HerveH44 Maybe keeler wasn't sure because there was no further response in the related discussion in one of the reviews (#990 (review))? |
I agree with the third category.
Re — unnecessary cicks, I'd just have a text field w/ autocomplete as an
alternative input.
…On Wed, 6 May 2020, 11:17 tooomm, ***@***.***> wrote:
@HerveH44 <https://github.com/HerveH44> Maybe keeler wasn't sure because
there was no further response in the related discussion? #990 (review)
<#990 (review)>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#990 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AK2DJQZGOH263YEZ7IMFPS3RQEMH7ANCNFSM4MQ4Q3OQ>
.
|
@HerveH44 Are you referring to this thread? #990 (comment) If that's the thread you mean, I did attempt to address this by adding delegate functions for regular and decadent draft picks, though it's arguably not ideal. I didn't feel comfortable just marking it resolved like some of your other feedback which was much more straightforward (e.g. use lodash functions). If that's not the thread you're talking about I'm having trouble finding what thread you mean, this PR is getting a bit cluttered. |
Hi @keeler, sorry for the misunderstanding. The PR is really amazing and will help us integrating the new mods in the future. Thanks for your contribution. |
Summary
Fix #919
Adds a "Decadent" game mode to address the request in #919. As described in that issue, it's also known as "Rich Man's Draft" or "Decadent Sealed".
This is essentially a draft where each person picks only one card from a pack, discards the rest, and opens a new pack until all packs have been drafted. As such, it's implemented as a draft where if you would pass, a new round is started instead.
Details
frontend/src/utils.jsx
to split out common components into thefrontend/src/components/
directory.Checkbox
,Spaced
,Select
,TextArea
.App.save("list", ...)
-->App.save(link, ...)
utils.jsx
-->utils.js
frontend/test/
for tests oftoTitleCase()
in utils.js.SetReplicated
component which is like theSet
component except it fills the entire set array with the same set instead of handling one index of the array.SetChoices
which is reused inSet
andSetReplicated
.